- 
                Notifications
    
You must be signed in to change notification settings  - Fork 1.2k
 
mspm0/gpio: use maitake-sync for gpio wakers #4758
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Fixes embassy-rs#4015 hawkw/mycelium#555 must be merged first. I have verified this new setup works correctly on C1104. I will test on G3507 and maybe L2228 tomorrow. As discussed in embassy-rs#4015, maitake-sync allows the cost of GPIO wakers to be paid by usage rather than statically, even if most pins are unused. Currently this disables the old method of statically allocated wakers. I would like some discussion whether this would be a good idea to mark as a choice (although I want to spin up an example using many pins to test this). Here I have some numbers for RAM and flash usage on MSPM0C1104, for which statically allocated wakers waste 25% of RAM. For C1104, the blinky example is freed up nearly 25% of RAM. Unfortunately this increases the `.text` size. I suspect some optimization of what is inlined may help here. Before waitmap (blinky): text data bss dec hex filename 3944 80 408 4432 1150 blinky i509vcb@verdigris:~/Dev/embassy/examples/mspm0c1104$ cargo objdump --release --bin blinky -- -C -t | grep '.bss' Finished `release` profile [optimized + debuginfo] target(s) in 0.05s 20000050 l O .bss 00000040 blinky::__embassy_main::POOL::hdf4890576116d2f8 20000098 l O .bss 00000100 embassy_mspm0::gpio::PORTA_WAKERS::h3e46d7edcc235649 20000090 l O .bss 00000008 embassy_mspm0::dma::STATE::hc0d1f2780f4fe38c 20000199 l O .bss 00000001 defmt_rtt::RTT_ENCODER::h5651992cc6ed4618 (.0) 2000019a l O .bss 00000001 defmt_rtt::RTT_ENCODER::h5651992cc6ed4618 (.1) 2000019b l O .bss 00000001 defmt_rtt::RTT_ENCODER::h5651992cc6ed4618 (.2) 2000019c l O .bss 00000001 defmt_rtt::RTT_ENCODER::h5651992cc6ed4618 (.3) 2000019d l O .bss 00000001 defmt_rtt::RTT_ENCODER::h5651992cc6ed4618 (.4) 20000198 g O .bss 00000001 _EMBASSY_DEVICE_PERIPHERALS 20000050 g .bss 00000000 __sbss 200001a0 g .bss 00000000 __ebss After waitmap (blinky): text data bss dec hex filename 4228 80 164 4472 1178 blinky i509vcb@verdigris:~/Dev/embassy/examples/mspm0c1104$ cargo objdump --release --bin blinky -- -C -t | grep '.bss' Finished `release` profile [optimized + debuginfo] target(s) in 0.05s 20000050 l O .bss 00000040 blinky::__embassy_main::POOL::h6016dabb69486a46 20000090 l O .bss 00000014 embassy_mspm0::gpio::GPIO_WAIT_MAP::ha809843ed7743bb9 200000a5 l O .bss 00000001 defmt_rtt::RTT_ENCODER::h7afe18b11b883426 (.0) 200000a6 l O .bss 00000001 defmt_rtt::RTT_ENCODER::h7afe18b11b883426 (.1) 200000a7 l O .bss 00000001 defmt_rtt::RTT_ENCODER::h7afe18b11b883426 (.2) 200000a8 l O .bss 00000001 defmt_rtt::RTT_ENCODER::h7afe18b11b883426 (.3) 200000a9 l O .bss 00000001 defmt_rtt::RTT_ENCODER::h7afe18b11b883426 (.4) 200000a4 g O .bss 00000001 _EMBASSY_DEVICE_PERIPHERALS 20000050 g .bss 00000000 __sbss 200000ac g .bss 00000000 __ebss Before waitmap (button): text data bss dec hex filename 4100 80 408 4588 11ec button i509vcb@verdigris:~/Dev/embassy/examples/mspm0c1104$ cargo objdump --release --bin button -- -C -t | grep '.bss' Finished `release` profile [optimized + debuginfo] target(s) in 0.05s 20000098 l O .bss 00000100 embassy_mspm0::gpio::PORTA_WAKERS::h3e46d7edcc235649 20000050 l O .bss 00000040 button::__embassy_main::POOL::ha91ef408a93b6f8f 20000090 l O .bss 00000008 embassy_mspm0::dma::STATE::hc0d1f2780f4fe38c 20000199 l O .bss 00000001 defmt_rtt::RTT_ENCODER::h5651992cc6ed4618 (.0) 2000019a l O .bss 00000001 defmt_rtt::RTT_ENCODER::h5651992cc6ed4618 (.1) 2000019b l O .bss 00000001 defmt_rtt::RTT_ENCODER::h5651992cc6ed4618 (.2) 2000019c l O .bss 00000001 defmt_rtt::RTT_ENCODER::h5651992cc6ed4618 (.3) 2000019d l O .bss 00000001 defmt_rtt::RTT_ENCODER::h5651992cc6ed4618 (.4) 20000198 g O .bss 00000001 _EMBASSY_DEVICE_PERIPHERALS 20000050 g .bss 00000000 __sbss 200001a0 g .bss 00000000 __ebss After waitmap (button): text data bss dec hex filename 5544 80 236 5860 16e4 button i509vcb@verdigris:~/Dev/embassy/examples/mspm0c1104$ cargo objdump --release --bin button -- -C -t | grep '.bss' Finished `release` profile [optimized + debuginfo] target(s) in 0.05s 20000050 l O .bss 00000088 button::__embassy_main::POOL::h1a31a9cd1f4fa9de 200000d8 l O .bss 00000001 defmt_rtt::RTT_ENCODER::h7afe18b11b883426 (.0) 200000d9 l O .bss 00000001 defmt_rtt::RTT_ENCODER::h7afe18b11b883426 (.1) 200000da l O .bss 00000001 defmt_rtt::RTT_ENCODER::h7afe18b11b883426 (.2) 200000db l O .bss 00000001 defmt_rtt::RTT_ENCODER::h7afe18b11b883426 (.3) 200000dc l O .bss 00000001 defmt_rtt::RTT_ENCODER::h7afe18b11b883426 (.4) 200000e0 l O .bss 00000014 embassy_mspm0::gpio::GPIO_WAIT_MAP::ha809843ed7743bb9 20000050 g .bss 00000000 __sbss 200000f4 g .bss 00000000 __ebss 200000dd g O .bss 00000001 _EMBASSY_DEVICE_PERIPHERALS
| 
           stm32 has kind of the same problem. My idea to fix it was to make EXTI also use  This has other advantages, for example the HAL doesn't force registering all interrupts so you can use one pin with async and another pin with manual interrupts, which is a recurring ask from users. Have you considered whether this would work for mpsm0 as well?  | 
    
          
 Yes longer term it should probably be possible to let the gpio interrupts get bound individually so that the user can pick a handler depending on the pin. Where this is kind of complicated for TI is that some chips use an interrupt group for GPIO. This might mean the mspm0 hal would need some hacky bits to handle that. At least so far it is consistent whether a chip has dedicated interrupts or uses an interrupt group (no mix of both). Even with your suggestion, I still see good reason to consider using maitake-sync: I consider letting the handler being configurable and how wakers are implemented internally to be two different problems. An example is waiting for a pair of pins, one at a time in a loop. With maitake-sync you only need space for one waiter in the future state machine. Statically allocated wakers would appear more consistent, but only using 1 waker at a time means 8 bytes get wasted, as both pin wakers are disjoint in usage.  | 
    
Fixes #4015
hawkw/mycelium#555 must be merged first.
I have verified this new setup works correctly on C1104. I will test on G3507 and maybe L2228 tomorrow. Keeping this a draft till the dependent pull request is merged.
I also consider this pull request to be a testbed for whether other embassy-hals may benefit from using maitake-sync, specifically
WakeMap.As discussed in #4015, maitake-sync allows the cost of GPIO wakers to be paid by usage rather than statically, even if most pins are unused. Currently this disables the old method of statically allocated wakers. I would like some discussion whether this would be a good idea to mark as a choice (although I want to spin up an example using many pins to test this).
Here I have some numbers for RAM and flash usage on MSPM0C1104, for which statically allocated wakers waste 25% of RAM. For C1104, the blinky example is freed up nearly 25% of RAM. Unfortunately this increases the
.textsize. I suspect some optimization of what is inlined may help here.Before waitmap (blinky):
i509vcb@verdigris:~/Dev/embassy/examples/mspm0c1104$ cargo objdump --release --bin blinky -- -C -t | grep '.bss'
Finished
releaseprofile [optimized + debuginfo] target(s) in 0.05sAfter waitmap (blinky):
i509vcb@verdigris:~/Dev/embassy/examples/mspm0c1104$ cargo objdump --release --bin blinky -- -C -t | grep '.bss'
Finished
releaseprofile [optimized + debuginfo] target(s) in 0.05sBefore waitmap (button):
i509vcb@verdigris:~/Dev/embassy/examples/mspm0c1104$ cargo objdump --release --bin button -- -C -t | grep '.bss'
Finished
releaseprofile [optimized + debuginfo] target(s) in 0.05sAfter waitmap (button):
i509vcb@verdigris:~/Dev/embassy/examples/mspm0c1104$ cargo objdump --release --bin button -- -C -t | grep '.bss'
Finished
releaseprofile [optimized + debuginfo] target(s) in 0.05s@jamesmunns would appreciate review from you